Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feat] - Add Sitemap Feature for the Docs #170

Merged
merged 3 commits into from
Sep 13, 2023

Conversation

adithyaakrishna
Copy link
Contributor

Description:

  • This PR adds a sitemap feature for the docs to improve SEO

Signed-off-by: Adithya Krishna <[email protected]>
Signed-off-by: Adithya Krishna <[email protected]>
Signed-off-by: Adithya Krishna <[email protected]>
Copy link
Collaborator

alabulei1 commented Sep 8, 2023

Hello, I am a code review bot on flows.network. Here are my reviews of code commits in this PR.


Summary:

The pull request titled "[Feat] - Add Sitemap Feature for the Docs" includes a set of changes related to the sitemap configuration for the documentation.

Potential issues and errors include:

  • The sitemap configuration is hard-coded in the docusaurus.config.js file, which may need to be manually updated in the future.
  • The removal of the @docusaurus/preset-classic plugin without explanation, which could potentially affect the website's functionality.
  • The duplication of the sitemap configuration in the themeConfig object and the presets array, which may cause conflicts.
  • The change of the "Sitemap" link's to property to an href property with a hardcoded URL, which may cause problems if the sitemap URL changes.

The most important findings are:

  • The addition of the sitemap configuration and the inclusion of a link to the sitemap in the navbar.
  • The fixed formatting issues in the mediapipe.md file, improving the readability of the documentation.

Overall, while the addition of the sitemap feature is valuable, further clarification and investigation are needed regarding the potential issues and errors mentioned above.

Details

Commit 50f357e369c536c3abeb13c49a57b6b03488e2c8

Key changes in the pull request:

  • Fixed formatting issues in the mediapipe.md file in both English and Chinese documentation.
  • The changes included fixing line breaks, removing unnecessary line breaks, and removing duplicated content.

Potential problems:

  • No potential problems were identified in this pull request.
  • The changes seem to be minor formatting improvements and do not introduce any functional changes or issues.

Commit 329182065bcba0c4314c739871acb246f0750ecd

Key Changes:

  • Added sitemap configuration to the docusaurus.config.js file.
  • Modified the navbar configuration to include a link to the sitemap.

Potential Problems:

  • There don't seem to be any major problems with these changes. The sitemap configuration is properly added and the link to the sitemap is included in the navbar.
  • However, it's worth noting that the sitemap configuration is hard-coded in the docusaurus.config.js file and may need to be updated manually if there are changes in the future. It would be better to have a more dynamic approach for the sitemap configuration.

Commit 0244b7e737aea0a91e1a64b35845a01398da2c6b

Key Changes:

  • The sitemap configuration in the docusaurus.config.js file has been updated.
  • The @docusaurus/preset-classic plugin has been removed from the plugins array.
  • The sitemap configuration has been moved to the themeConfig object in the presets array.
  • The to property of the "Sitemap" link in the header has been changed to an href property.

Potential Problems:

  • The @docusaurus/preset-classic plugin has been removed without any explanation. It's unclear why this change was made and if it will affect the functionality of the website.
  • The sitemap configuration has been duplicated in two places: the themeConfig object and the presets array. This may cause confusion and potential conflicts.
  • The to property of the "Sitemap" link in the header has been changed to an href property, but the new value is a hardcoded URL. This may cause problems if the URL of the sitemap changes in the future.
  • There are some lines that have been removed from the file, but it's not clear if those changes are intentional or accidental.

Overall, the key changes seem to be related to the sitemap configuration. However, there are a few potential problems that need clarification and further investigation.

@adithyaakrishna
Copy link
Contributor Author

@q82419 Could you please confirm regarding this, https://github.com/WasmEdge/docs/pull/170/files#r1324101048? If its not needed, I will go ahead and remove it :)

@alabulei1 alabulei1 merged commit 256c116 into WasmEdge:main Sep 13, 2023
6 checks passed
@alabulei1
Copy link
Collaborator

@q82419 Could you please confirm regarding this, https://github.com/WasmEdge/docs/pull/170/files#r1324101048? If its not needed, I will go ahead and remove it :)

It's not the point. I merged your PR since sitemap is an important feature.

@adithyaakrishna
Copy link
Contributor Author

@alabulei1 Ohhh, I got it now, I guess you meant to say to remove the changes from this PR? 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants